Skip to content

Conversation

GromNaN
Copy link
Member

@GromNaN GromNaN commented Sep 26, 2025

Q A
Type feature
BC Break no
Fixed issues PHPORM-321 Fix #2828

Summary

Shamelessly copied from the brilliant work of Benjamin Eberlei on ORM:

@GromNaN GromNaN changed the title Add support for PHP 8.4 Lazy Objects RFC with configuration flag Add support for PHP 8.4 Lazy Objects with configuration flag Sep 26, 2025
@GromNaN GromNaN force-pushed the PHPORM-321 branch 3 times, most recently from 421e842 to cb0b5f6 Compare September 28, 2025 12:26
@GromNaN GromNaN requested a review from Copilot October 14, 2025 10:40
@GromNaN GromNaN marked this pull request as ready for review October 14, 2025 10:40
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for PHP 8.4 Lazy Objects with a configuration flag, implementing native lazy loading functionality as an alternative to the existing proxy mechanisms. The implementation borrows from the Doctrine ORM approach and introduces a new PropertyAccessor system to handle property access uniformly across different scenarios.

  • Introduces native lazy objects support for PHP 8.4+ as a replacement for existing proxy systems
  • Replaces the reflection-based property access with a new PropertyAccessor architecture
  • Adds comprehensive test coverage for property hooks and lazy object functionality

Reviewed Changes

Copilot reviewed 46 out of 46 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
lib/Doctrine/ODM/MongoDB/Configuration.php Adds configuration methods for enabling native lazy objects
lib/Doctrine/ODM/MongoDB/Proxy/Factory/NativeLazyObjectFactory.php New factory for creating native lazy objects in PHP 8.4+
lib/Doctrine/ODM/MongoDB/Mapping/PropertyAccessors/*.php New PropertyAccessor system replacing direct reflection usage
lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php Updated to use PropertyAccessor instead of reflection fields
lib/Doctrine/ODM/MongoDB/UnitOfWork.php Updated for native lazy object support and PropertyAccessor usage
tests/Documents84/ New test documents for PHP 8.4 property hooks functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@GromNaN GromNaN requested review from beberlei and malarzm October 14, 2025 11:13
@GromNaN GromNaN added this to the 2.14.0 milestone Oct 20, 2025
@GromNaN GromNaN changed the base branch from 2.13.x to 2.14.x October 20, 2025 14:59
Comment on lines 20 to 24
if ($this->dm->getConfiguration()->isNativeLazyObjectsEnabled()) {
return;
}

$this->markTestSkipped('Property hooks require native lazy objects to be enabled.');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did phpcs ask for the early return? I find the following easier to read...

Suggested change
if ($this->dm->getConfiguration()->isNativeLazyObjectsEnabled()) {
return;
}
$this->markTestSkipped('Property hooks require native lazy objects to be enabled.');
if ($this->dm->getConfiguration()->isNativeLazyObjectsEnabled()) {
$this->markTestSkipped('Property hooks require native lazy objects to be enabled.');
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, PHPCS. I hate this rule. We need a way to disable it when the condition only contains 1 statement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EarlyExit sniff supports the ignoreStandaloneIfInScope, ignoreOneLineTrailingIf, and ignoreTrailingIfWithOneInstruction options that allow us to customise this. I believe the ignoreTrailingIfWithOneInstruction option allows this kind of usage, but I also don't mind enabling all of them.

@GromNaN GromNaN requested review from alcaeus and Copilot October 22, 2025 11:03
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 49 out of 49 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in one of my projects, works fine with readonly properties and such. Great work!

@GromNaN GromNaN merged commit 63f6593 into doctrine:2.14.x Oct 22, 2025
24 checks passed
@GromNaN GromNaN deleted the PHPORM-321 branch October 22, 2025 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate to Native Lazy Objects

2 participants